fix(respect): secrets masking for encoded occurrences in har-output - #2998
Conversation
🦋 Changeset detectedLatest commit: acf3993 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
a696d78 to
fe080d1
Compare
Performance Benchmark (Lower is Faster)
|
There was a problem hiding this comment.
Pull request overview
This PR improves secret masking in @redocly/respect-core logger output so that secrets are also masked when they appear in encoded forms (e.g., JSON-escaped or URL/form-urlencoded) within HAR captures.
Changes:
- Add secret “variant” collection (raw, JSON-escaped, URL-encoded, form-urlencoded) and mask using those patterns.
- Refactor string masking to apply consistently for both string targets and recursively-walked object values.
- Add tests covering encoded secret occurrences and HAR-wide masking, plus a changeset for patch releases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/respect-core/src/modules/logger-output/mask-secrets.ts | Adds encoded-variant pattern collection and updates masking to replace all occurrences across strings and nested objects. |
| packages/respect-core/src/modules/tests/logger-output/mask-secrets.test.ts | Adds coverage for URL-encoded, JSON-escaped, repeated-occurrence, and HAR-capture masking scenarios. |
| .changeset/mask-encoded-secrets.md | Publishes a patch changeset for the masking fix. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/respect-core/src/modules/logger-output/mask-secrets.ts:30
- When multiple secrets (or their encoded variants) overlap (e.g., one is a substring of another), iterating patterns in insertion order can partially mask the longer secret first, leaving a suffix visible (e.g. replacing
abcbeforeabcdyields********d). Sorting patterns by length (descending) before masking avoids this leakage and makes masking deterministic.
return Array.from(patterns);
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/respect-core/src/modules/logger-output/mask-secrets.ts:23
collectSecretPatternsskips only empty strings, but a whitespace-only secret (e.g.' ') can still be added tosecretsSetby other parts of respect-core. That would cause masking to replace spaces (and potentially+in the form-urlencoded variant) across the entire output, making logs unusable. Treat whitespace-only secrets as empty and skip them when building patterns.
for (const secret of secretsSet) {
if (!secret) continue;
patterns.add(secret);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/respect-core/src/modules/logger-output/mask-secrets.ts:29
collectSecretPatternscallsencodeURIComponent(secret)/URLSearchParams(...).toString()without guarding againstURIError(thrown when the string contains malformed UTF-16 surrogate pairs). Since these secrets originate from captured traffic and headers, a single malformed value could crash masking/logging entirely. Consider wrapping the encoding steps in a try/catch and skipping the encoded variants on failure (the raw secret pattern will still be masked).
patterns.add(secret);
patterns.add(JSON.stringify(secret).slice(1, -1));
patterns.add(encodeURIComponent(secret));
// URLSearchParams serializes to `secret=value`; drop the key to keep
// the form-urlencoded variant (spaces become `+`, unlike encodeURIComponent).
patterns.add(new URLSearchParams([['secret', secret]]).toString().slice('secret='.length));
}
|
What about the |
DmitryAnansky
changed the title
The
cc: @sobanieca-redocly , @Redocly/cyberpunks |
ebc7074 to
18b17ff
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit acf3993. Configure here.
What/Why/How?
Fixed secrets masking to cover encoded occurrences of a secret in har-output.
Reference
Testing
Screenshots (optional)
Check yourself
Security
Note
Low Risk
Localized improvement to secret redaction in logging/HAR output with broad test coverage; no auth or data-path changes beyond safer masking.
Overview
Secrets masking in
@redocly/respect-corenow replaces encoded forms of known secrets, not only literal substrings—so values like URL-encoded form bodies, JSON-escaped strings, and nested HAR request/response text are redacted inhar-output.maskSecretsbuilds a pattern list per secret (raw value, JSON string escape body,encodeURIComponent, andURLSearchParamsform style), skips whitespace-only entries, avoidsencodeURIComponenton ill-formed strings (lone surrogates), and applies longest-match-first so a shorter secret cannot leave part of a longer one visible. Tests cover HAR captures, multiple occurrences in one string, and edge cases.Reviewed by Cursor Bugbot for commit acf3993. Bugbot is set up for automated code reviews on this repo. Configure here.